apps/web/components/booking/CancelBooking.tsx (1)

259-261: Disable conditions are correctly combined

Button disables when host reason is missing or the attendee hasn’t acknowledged the fee. Matches the earlier intent.

Also applies to: 160-164

apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (1)

182-194: Select now includes appId; add deterministic ordering to avoid picking an arbitrary payment.

Without an orderBy, findFirst may return a stale attempt; this can mis-drive appId-based logic/UI.

Apply:

   const payment = await prisma.payment.findFirst({
     where: {
       bookingId: bookingInfo.id,
     },
+    orderBy: { createdAt: "desc" },
     select: {
       appId: true,
       success: true,
       refunded: true,
       currency: true,
       amount: true,
       paymentOption: true,
     },
   });

If unsure Payment has createdAt, search the Prisma schema to confirm and adjust the field if named differently.

packages/features/bookings/lib/getBookingToDelete.ts (2)

25-26: Confirm locale is consumed; drop if unused to keep payload lean.

---

50-51: teamId duplication—ensure it’s needed beyond team.id; otherwise remove.

packages/app-store/stripepayment/zod.ts (1)

17-17: Tighten schema: integer, min bound, and require value/unit when auto-charge is enabled.

Prevents misconfiguration and aligns with UI intent.

Apply:

-export const autoChargeNoShowFeeTimeUnitEnum = z.enum(["minutes", "hours", "days"]);
+export const autoChargeNoShowFeeTimeUnitEnum = z.enum(["minutes", "hours", "days"]);
@@
-    autoChargeNoShowFeeIfCancelled: z.boolean().optional(),
-    autoChargeNoShowFeeTimeValue: z.number().optional(),
-    autoChargeNoShowFeeTimeUnit: autoChargeNoShowFeeTimeUnitEnum.optional(),
+    autoChargeNoShowFeeIfCancelled: z.boolean().optional(),
+    autoChargeNoShowFeeTimeValue: z.number().int().min(1).optional(),
+    autoChargeNoShowFeeTimeUnit: autoChargeNoShowFeeTimeUnitEnum.optional(),
-  })
-);
+  }).superRefine((data, ctx) => {
+    if (data.autoChargeNoShowFeeIfCancelled) {
+      if (data.autoChargeNoShowFeeTimeValue == null) {
+        ctx.addIssue({ code: z.ZodIssueCode.custom, path: ["autoChargeNoShowFeeTimeValue"], message: "required" });
+      }
+      if (data.autoChargeNoShowFeeTimeUnit == null) {
+        ctx.addIssue({ code: z.ZodIssueCode.custom, path: ["autoChargeNoShowFeeTimeUnit"], message: "required" });
+      }
+    }
+  })
+);

Also applies to: 28-31

packages/app-store/stripepayment/components/EventTypeAppSettingsInterface.tsx (2)

80-85: Typo: autoChange → autoCharge.

Keeps naming consistent with feature terminology.

Apply:

-  const autoChangeTimeUnitOptions = [
+  const autoChargeTimeUnitOptions = [
     { value: autoChargeNoShowFeeTimeUnitEnum.enum.minutes, label: t("minutes") },
     { value: autoChargeNoShowFeeTimeUnitEnum.enum.hours, label: t("hours") },
     { value: autoChargeNoShowFeeTimeUnitEnum.enum.days, label: t("days") },
   ];

And update references below accordingly.

---

221-268: Make inputs fully controlled, avoid NaN, and honor disabled state on the checkbox.

Prevents React controlled/uncontrolled warnings and accidental NaN in app data.

Apply:

-                <CheckboxField
-                  checked={autoChargeNoShowFeeIfCancelled}
-                  onChange={(e) => setAppData("autoChargeNoShowFeeIfCancelled", e.target.checked)}
-                  description={t("auto_charge_for_last_minute_cancellation")}
-                />
+                <CheckboxField
+                  checked={autoChargeNoShowFeeIfCancelled}
+                  disabled={disabled}
+                  onChange={(e) => setAppData("autoChargeNoShowFeeIfCancelled", e.target.checked)}
+                  description={t("auto_charge_for_last_minute_cancellation")}
+                />
@@
-                    <TextField
+                    <TextField
                       labelSrOnly
                       type="number"
                       className={classNames(
                         "border-default my-0 block w-16 text-sm [appearance:textfield] ltr:mr-2 rtl:ml-2"
                       )}
                       placeholder="2"
                       disabled={disabled}
-                      min={0}
-                      defaultValue={autoChargeNoShowFeeTimeValue}
+                      min={0}
                       required={autoChargeNoShowFeeIfCancelled}
                       value={autoChargeNoShowFeeTimeValue ?? ""}
-                      onChange={(e) =>
-                        setAppData("autoChargeNoShowFeeTimeValue", parseInt(e.currentTarget.value))
-                      }
+                      onChange={(e) => {
+                        const v = e.currentTarget.value;
+                        setAppData(
+                          "autoChargeNoShowFeeTimeValue",
+                          v === "" ? undefined : parseInt(v, 10)
+                        );
+                      }}
                     />
@@
-                    <Select
-                      options={autoChangeTimeUnitOptions}
+                    <Select
+                      options={autoChargeTimeUnitOptions}
                       isSearchable={false}
                       isDisabled={disabled}
                       onChange={(option) => setAppData("autoChargeNoShowFeeTimeUnit", option?.value)}
-                      value={
-                        autoChangeTimeUnitOptions.find((opt) => opt.value === autoChargeNoShowFeeTimeUnit) ||
-                        autoChangeTimeUnitOptions[0]
-                      }
-                      defaultValue={
-                        autoChangeTimeUnitOptions.find((opt) => opt.value === autoChargeNoShowFeeTimeUnit) ||
-                        autoChangeTimeUnitOptions[0]
-                      }
+                      value={
+                        autoChargeTimeUnitOptions.find((opt) => opt.value === autoChargeNoShowFeeTimeUnit) ||
+                        autoChargeTimeUnitOptions[0]
+                      }
                     />

packages/lib/payment/shouldChargeNoShowCancellationFee.ts (1)

24-35: Type access on metadata is overly complex; simplify the indexing.

Reduces casts and improves readability.

Apply:

-  const cancellationFeeEnabled =
-    eventTypeMetadata?.apps?.[paymentAppId as keyof typeof eventTypeMetadata.apps]
-      ?.autoChargeNoShowFeeIfCancelled;
-  const paymentOption =
-    eventTypeMetadata?.apps?.[paymentAppId as keyof typeof eventTypeMetadata.apps]?.paymentOption;
-  const timeValue =
-    eventTypeMetadata?.apps?.[paymentAppId as keyof typeof eventTypeMetadata.apps]
-      ?.autoChargeNoShowFeeTimeValue;
-  const timeUnit =
-    eventTypeMetadata?.apps?.[paymentAppId as keyof typeof eventTypeMetadata.apps]
-      ?.autoChargeNoShowFeeTimeUnit;
+  const appCfg = (eventTypeMetadata?.apps as Record<string, any> | undefined)?.[paymentAppId];
+  const cancellationFeeEnabled = appCfg?.autoChargeNoShowFeeIfCancelled;
+  const paymentOption = appCfg?.paymentOption;
+  const timeValue = appCfg?.autoChargeNoShowFeeTimeValue;
+  const timeUnit = appCfg?.autoChargeNoShowFeeTimeUnit as string | undefined;

packages/lib/payment/shouldChargeNoShowCancellationFee.test.ts (2)

14-14: Align test name with function under test.

-describe("shouldChargeCancellationFee", () => {
+describe("shouldChargeNoShowCancellationFee", () => {

---

167-187: Add boundary test for equality at threshold.

Covers now === threshold to lock behavior.

   it("should return false when timeUnit is missing", () => {
     const mockEventTypeMetadata = {
       apps: {
         stripe: {
           autoChargeNoShowFeeIfCancelled: true,
           paymentOption: "HOLD",
           autoChargeNoShowFeeTimeValue: 24,
         },
       },
     };

     const result = shouldChargeNoShowCancellationFee({
       eventTypeMetadata: mockEventTypeMetadata as EventTypeMetadata,
       booking: { startTime: new Date("2024-09-01T12:00:00Z") },
       payment: mockPayment,
     });

     expect(result).toBe(false);
   });
+
+  it("should return true when cancelled exactly at the threshold", () => {
+    // now = 2024-09-01T10:00:00Z (set in beforeAll), threshold = start - 2h
+    const startTime = new Date("2024-09-01T12:00:00Z");
+    const result = shouldChargeNoShowCancellationFee({
+      eventTypeMetadata: mockEventTypeMetadata as EventTypeMetadata,
+      booking: { startTime },
+      payment: mockPayment,
+    });
+    expect(result).toBe(true);
+  });
 });

apps/web/public/static/locales/en/common.json (1)

3614-3617: Standardize “no‑show” hyphenation for consistency.

Other strings (e.g., "No-show Fee") use a hyphen.

-  "cancel_booking_acknowledge_no_show_fee": "I acknowledge that by cancelling the booking within {{timeValue}} {{timeUnit}} of the start time I will be charged the no show fee of {{amount, currency}}",
+  "cancel_booking_acknowledge_no_show_fee": "I acknowledge that by cancelling the booking within {{timeValue}} {{timeUnit}} of the start time I will be charged the no-show fee of {{amount, currency}}",

packages/features/bookings/lib/handleCancelBooking.ts (1)

581-581: Prefer named export.

Switch to export { handler } for clearer imports and better tree-shaking. Low priority due to widespread call sites.

packages/features/bookings/lib/handleCancelBooking/test/handleCancelBooking.test.ts (4)

777-780: Freeze time to make threshold assertions deterministic.

Use vi.setSystemTime so the 30-minute window is stable across environments.

-    const now = new Date();
-    const startTime = new Date(now.getTime() + 30 * 60 * 1000);
+    const now = new Date("2025-01-01T10:00:00.000Z");
+    vi.setSystemTime(now);
+    const startTime = new Date(now.getTime() + 30 * 60 * 1000);

---

759-763: Remove unsupported id from getBooker call.

getBooker doesn’t accept id; this arg is ignored and can confuse future readers.

-    const booker = getBooker({
-      email: "booker@example.com",
-      name: "Booker",
-      id: 999,
-    });
+    const booker = getBooker({ email: "booker@example.com", name: "Booker" });

---

861-869: Assert that a charge actually happened.

Add a DB assertion that the HOLD payment was captured (success === true) to prevent false positives.

     const result = await handleCancelBooking({
       bookingData: {
         id: idOfBookingToBeCancelled,
         uid: uidOfBookingToBeCancelled,
         cancelledBy: booker.email,
         cancellationReason: "Attendee cancelled within time threshold",
       },
       userId: 999,
     });
 
     expect(result.success).toBe(true);
+    // Verify capture
+    const payment = await prisma.payment.findFirst({
+      where: { bookingId: idOfBookingToBeCancelled },
+      orderBy: { id: "desc" },
+      select: { success: true },
+    });
+    expect(payment?.success).toBe(true);

---

513-623: Add boundary/unit coverage for cancellation window.

Consider tests for: exactly-on-threshold, minutes and days units, and DST transitions.

I can add cases for MINUTES/HOURS/DAYS and a DST-crossing startTime to harden the logic.

Also applies to: 624-755

packages/trpc/server/routers/viewer/payments/chargeCard.handler.ts (2)

18-22: Use TRPCError(NOT_FOUND) for missing booking.

Surface a proper TRPC error to clients.

-  if (!booking) {
-    throw new Error("Booking not found");
-  }
+  if (!booking) {
+    throw new TRPCError({ code: "NOT_FOUND", message: "Booking not found" });
+  }

---

37-41: Preserve error details when available.

Bubble up TRPCError instances; only wrap unknowns.

-  } catch (error) {
-    console.error(error);
-    throw new TRPCError({
-      code: "INTERNAL_SERVER_ERROR",
-      message: `Failed to charge no show fee for ${booking.id}`,
-    });
-  }
+  } catch (error) {
+    console.error(error);
+    if (error instanceof TRPCError) throw error;
+    throw new TRPCError({
+      code: "INTERNAL_SERVER_ERROR",
+      message: `Failed to charge no show fee for ${booking.id}`,
+    });
+  }

apps/web/components/booking/__tests__/CancelBooking.cancellationFee.test.tsx (5)

28-43: Remove unused next-i18next mock to reduce noise.

CancelBooking uses useLocale; this mock isn’t consumed here.

-vi.mock("next-i18next", () => ({
-  useTranslation: () => ({
-    t: (key: string, options?: Record<string, unknown>) => {
-      if (key === "cancellation_fee_warning_cancel") {
-        const opts = options as {
-          time?: string;
-          unit?: string;
-          amount?: number;
-          formatParams?: { amount?: { currency?: string } };
-        };
-        return `Cancelling within ${opts?.time} ${opts?.unit} will result in a ${opts?.amount} ${opts?.formatParams?.amount?.currency} cancellation fee being charged to your card.`;
-      }
-      return key;
-    },
-  }),
-}));
+// next-i18next not needed in this test; UI uses useLocale()

---

106-115: Rename misleading fixture; it actually has fee enabled.

The object named “WithoutFee” still sets autoChargeNoShowFeeIfCancelled: true, which confuses test intent.

-const mockEventTypeMetadataWithoutFee = {
+const mockEventTypeMetadataOutsideThreshold = {
   apps: {
     stripe: {
       autoChargeNoShowFeeIfCancelled: true,
       autoChargeNoShowFeeTimeValue: 1,
       autoChargeNoShowFeeTimeUnit: "hours" as const,
       paymentOption: "HOLD" as const,
     },
   },
 };
...
-        eventTypeMetadata={mockEventTypeMetadataWithoutFee}
+        eventTypeMetadata={mockEventTypeMetadataOutsideThreshold}

Also applies to: 171-179

---

163-166: Make assertion robust to singular/plural (“1 hour” vs “1 hours”).

Avoid hard-coding a potentially grammatically incorrect string.

-expect(
-  screen.getByText(/I acknowledge that cancelling within 1 hours will result in a/)
-).toBeInTheDocument();
+expect(
+  screen.getByText(/I acknowledge that cancelling within 1 hour(s)? will result in a/i)
+).toBeInTheDocument();

---

149-166: Also assert CTA gating (button disabled until acknowledged).

Covers the most important UX guard.

 render(
   <CancelBooking
@@
   />
 );
 
-expect(
-  screen.getByText(/I acknowledge that cancelling within 1 hour(s)? will result in a/i)
-).toBeInTheDocument();
+const ack = screen.getByText(/I acknowledge that cancelling within 1 hour\(s\)\? will result in a/i);
+expect(ack).toBeInTheDocument();
+const confirm = screen.getByTestId("confirm_cancel");
+expect(confirm).toBeDisabled();

---

13-26: Trim unused mocks (trpc/useRouter) for leaner tests.

They aren’t exercised by this component path.

-vi.mock("@calcom/trpc", () => ({ ... }))
-vi.mock("next/router", () => ({ ... }))
+// Remove unused mocks; keep test focused on CancelBooking branch

Also applies to: 72-77

packages/lib/payment/processNoShowFeeOnCancellation.ts (3)

29-41: Extend exemption to org-level admins (if team belongs to an org).

When teamId maps to an org, org ADMIN/OWNER should also bypass charges.

   if (cancelledByUserId && booking.eventType?.teamId) {
     const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({
       userId: cancelledByUserId,
       teamId: booking.eventType.teamId,
     });
 
     if (membership && (membership.role === "ADMIN" || membership.role === "OWNER")) {
       log.info(
         `Booking ${booking.uid} was cancelled by team admin/owner (${cancelledByUserId}), skipping no-show fee`
       );
       return;
     }
+
+    // Optional: check parent organization membership as an admin/owner
+    // const org = await new TeamRepository(prisma).findParentOrganizationByTeamId(booking.eventType.teamId);
+    // if (org) {
+    //   const orgMembership = await MembershipRepository.findUniqueByUserIdAndTeamId({
+    //     userId: cancelledByUserId,
+    //     teamId: org.id,
+    //   });
+    //   if (orgMembership && (orgMembership.role === "ADMIN" || orgMembership.role === "OWNER")) {
+    //     log.info(`Booking ${booking.uid} was cancelled by org admin/owner (${cancelledByUserId}), skipping no-show fee`);
+    //     return;
+    //   }
+    // }
   }

---

73-76: Preserve error stack; include cause.

Re-wrapping loses stack/context.

-  } catch (error) {
-    log.error(`Error charging no-show fee for booking ${booking.uid}`, error);
-    throw new Error(`Failed to charge no-show fee with error ${error}`);
-  }
+  } catch (error) {
+    log.error(`Error charging no-show fee for booking ${booking.uid}`, error);
+    // Prefer preserving original error; if wrapping, use cause
+    if (error instanceof Error) throw error;
+    throw new Error(`Failed to charge no-show fee`, { cause: error });
+  }

---

18-19: Add structured logs with booking/payment ids for observability.

Makes support/debugging easier.

-const log = logger.getSubLogger({ prefix: ["processNoShowFeeOnCancellation"] });
+const log = logger.getSubLogger({ prefix: [`[processNoShowFeeOnCancellation uid=${booking.uid}]`] });
@@
-  log.info(`Date is not valid for no-show fee to charge for booking ${booking.uid}`);
+  log.info(`Not charging no-show fee: outside threshold (bookingUid=${booking.uid})`);
@@
-    await handleNoShowFee({
+    log.info(`Charging no-show fee (bookingId=${booking.id}, paymentId=${paymentToCharge.id})`);
+    await handleNoShowFee({
       booking,
       payment: paymentToCharge,
     });

Also applies to: 62-66, 67-73

apps/web/playwright/cancellation-fee-warning.e2e.ts (1)

36-37: Prefer data-testid or more resilient assertions over text snippets.

Reduces i18n coupling and copy fragility. If adding testids isn’t feasible now, at least soften the matcher.

-await expect(page.getByText(/I acknowledge that if I cancel within 2 hours/)).toBeVisible();
+await expect(page.getByText(/I acknowledge that .* cancel within 2 hours/i)).toBeVisible();
@@
-await expect(page.getByText(/Cancelling within 24 hours will result in a/)).toBeVisible();
+await expect(page.getByText(/Cancelling within 24 hours/i)).toBeVisible();
@@
-await expect(page.getByText(/I acknowledge that if I cancel within/)).toBeHidden();
+await expect(page.getByText(/I acknowledge that .* cancel within/i)).toHaveCount(0);

Also applies to: 64-65, 92-92

apps/web/components/booking/CancelBooking.tsx (3)

137-143: Localize unit label instead of injecting raw enum values

Passing raw "minutes" | "hours" | "days" into a translation placeholder risks untranslated output. Map to i18n keys and pass the localized label.

@@
-const timeUnit = booking?.payment?.appId
-  ? (getAppMetadata(booking.payment.appId) as Record<string, unknown> | null)?.autoChargeNoShowFeeTimeUnit
-  : null;
+const unitKey = timeUnit ? ({ minutes: "minutes", hours: "hours", days: "days" } as const)[timeUnit] : null;
+const timeUnitLabel = unitKey ? t(unitKey) : "";
@@
-  description={t("cancel_booking_acknowledge_no_show_fee", {
-    timeValue,
-    timeUnit,
-    amount: booking.payment.amount / 100,
-    formatParams: { amount: { currency: booking.payment.currency } },
-  })}
+  description={t("cancel_booking_acknowledge_no_show_fee", {
+    timeValue,
+    timeUnit: timeUnitLabel,
+    amount: booking.payment.amount / 100,
+    formatParams: { amount: { currency: booking.payment.currency } },
+  })}

Also applies to: 233-249

---

233-249: Make the checkbox controlled for predictable UI state

Bind checked to state to avoid uncontrolled/controlled transitions and ensure SSR hydration consistency.

-<CheckboxField
+<CheckboxField
+  checked={acknowledgeCancellationNoShowFee}
   description={t("cancel_booking_acknowledge_no_show_fee", {
@@
   onChange={(e) => setAcknowledgeCancellationNoShowFee(e.target.checked)}

---

113-116: Prefer named export for components

For non-page components, named exports improve tree-shaking and refactors.

-export default function CancelBooking(props: Props) {
+export function CancelBooking(props: Props) {

And update imports accordingly.

packages/lib/payment/handleNoShowFee.ts (5)

65-66: Typo: eventTypeMetdata → eventTypeMetadata

Minor naming fix improves readability and prevents future confusion.

-  const eventTypeMetdata = eventTypeMetaDataSchemaWithTypedApps.parse(booking.eventType?.metadata ?? {});
+  const eventTypeMetadata = eventTypeMetaDataSchemaWithTypedApps.parse(booking.eventType?.metadata ?? {});
@@
-    await sendNoShowFeeChargedEmail(attendeesListPromises[0], evt, eventTypeMetdata);
+    await sendNoShowFeeChargedEmail(attendeesList[0], evt, eventTypeMetadata);

Also applies to: 167-167

---

67-71: Normalize precondition errors (consistency and localization)

Errors thrown before the try block won’t be localized. Either wrap preconditions in the same try/catch or throw ErrorWithCode so upstream can map them.

Option A (wrap in try/catch) or Option B (use ErrorWithCode with stable codes). Happy to draft either approach if you confirm the preferred pattern in this module.

Also applies to: 110-120, 140-155

---

3-5: Remove stray eslint-disable-next-line

The // eslint-disable-next-line before the import is unnecessary and suppresses useful checks.

-// eslint-disable-next-line
 import { PaymentServiceMap } from "@calcom/app-store/payment.services.generated";

---

95-103: Localize fallback organizer name

Hard-coded "Nameless" can leak into user emails. Prefer a localized fallback or the email local-part.

-      name: booking.user?.name || "Nameless",
+      name: booking.user?.name || tOrganizer("nameless_user"),

If nameless_user doesn’t exist, I can add it to locales.

---

159-179: Emit observability signal for success/failure

Add structured telemetry (success/failure, appId, paymentId, teamId/userId) for alerting and dashboards.

I can wire a logger.info + metrics counter here or integrate with your existing telemetry service if you point me to it.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.

Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.

Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In BookingPaymentInitiatedDTO and other webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking field is a restricted structure containing only specific fields (id, eventTypeId, userId) rather than the full database booking object, so there are no security or PII leakage concerns when passing the booking object to buildEventPayload.